Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Team-19][BE] Bongf/Tree 기능 구현 #75

Open
wants to merge 115 commits into
base: dahun-lee-daji
Choose a base branch
from

Conversation

choitree
Copy link

@choitree choitree commented Jun 4, 2021

안녕하세요 팀19 봉프, 트리입니다.
마지막 날이 되어서야 PR 요청드리게 되네요.
리뷰해주시면, 다음 프로젝트가 끝나고 수정 가능하겠지만 리뷰를 반영하는 것이 학습에 많은 도움이 되기 때문에
오늘이라도 PR을 보내기로 결정했습니다.
오오쓰는 아직 구현되지 못해서, 로그인이 필요한 부분에는 url에 userId를 넣도록 협의했습니다. 이 부분 참고 부탁드립니다.
또한, Host에 대한 테이블 분리가 필요하지만, Room안에 포함되어 있는 점도 참고해주세요.

추가로, RoomController에 포함된 get 요청에 대해서 설명을 덧붙입니다.

  • @GetMapping("/rooms/{roomId}/price")

    • 방을 예약 직전 단계에서 할인가, 세금 등이 붙은 요금을 보여주는 url입니다.
      image
  • @GetMapping("/rooms/price")

    • 검색 기능에서, 지도의 위도/경도를 기준으로 결제 가능한 방들의 가격 범위를 나타내주기 위한 url입니다.
    • 해당 response로 아래와 같은 그래프를 ios에서 그리도록 협의하였습니다.
      image
  • @GetMapping("/rooms")

    • 검색 기능에서 검색 조건이 입력되어서 최종 검색된 방들을 보여주기 위한 url입니다.
    • 기획서의 내용처럼, 건너뛰기가 적용되도록 한가지 조건이라도 검색 조건이 입력되면 방을 찾을 수 있고,
      아무런 조건이 없다면 예외 처리되도록 구현되어 있습니다.

PR이 늦어진 점 양해 부탁드리며, 리뷰 잘 부탁드립니다.☺️

ERD

image

배포 주소

http://52.79.226.11:8080

API 문서

https://documenter.getpostman.com/view/14935677/TzRa7j5h

dahun-lee-daji and others added 30 commits May 17, 2021 15:34
- MockAPI 생성

issue: #2
- 데이터 타입을 고려하여 데이터 타입 변경

issue: #8
- json에서 날짜 형식을 받기 위한 포맷팅 설정

issue :#8
- db 설계에 맞게 entity 생성

issue: #8
- 이전 변경 내용이 git Commit에 포함되지 않아 다시 커밋
- 컨트롤러를 분리한다
- build폴더를 gitignore 파일에 포함
- 트리가 맥을 사용하여gitignore파일 수정
- User가 booking과 whishlist를 List로 가질 수 있도록 관계 설정
- 테이블간의 관계를 list로 묶어준 내용을 반영하여 스키마 sql 변경
- 합의한 내용에 따라 entity변경
- 필드에 빠진 부분이 있어 스키마 sql수정
- Booking이 User의 List로 들어가있던 구조에서
- userId를 필드로 갖는 것으로 변경
- userId와 bookingId로 검색시 해당 user, booking을 찾지 못하는 상황에
  대한 예외 추가
bong6981 and others added 27 commits May 29, 2021 12:42
- Room 안에 필드를 묶어서 Embedded로 묶어줌
- RoomId로 Room을 찾아오는 기능을 구현
-현재 협의된 address를 조건으로 검색하는 api
-위도, 경도를 조건으로 +- 0.006 범주로 검색하는 api
-현재 협의된 address를 조건으로 검색하는 api
-위도, 경도를 조건으로 +- 0.006 범주로 검색하는 api
[BE] 가격범위 검색하는 기능 구현
-totalPrice를 제외한 부분 구현
-totalPrice 계산에 필요한 로직 추가하기 위해 먼저 커밋
-totalPrice를 부분 구현
-건너뛰기 하여도 검색 가능
- 예약 안된 방만 조회 및 totalPrice 확인 가능
[BE] 전체 기능 검색하는 기능 구현
- book, cancel 하는 service 메소드 명 변경
- 예약 취소시 해당 user인지 체크하는 로직 추가(booking.isUser)
- 권한이 없을시에 NotAuthorized 예외 추가
- 트리에게 질문할 것 : checkUser 이해 안갑니다 ㅠㅠ
- 사용하지 않는 클래스 삭제
- 사용자 정의 예외처리 추가
- 주석 삭제
- 사용하지 않는 클래스 삭제
- 코드 정렬(첫줄 띄어쓰기, 마지막줄 붙여쓰기)
- 주석 삭제
- Service에서 다른 DAO 호출이 아닌, 다른 Service 호출
- toString 삭제
- 사용하지 않는 getter, setter 삭제
@LarryJung LarryJung self-assigned this Jun 8, 2021
Copy link

@LarryJung LarryJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요! 마지막날에 올리셨지만 피드백을 드려야 도움이 되시니 챙겨봤습니다. ㅎㅎ

컨트롤러, 서비스 등 역할이 잘 나뉘어 있어 잘 읽히는 코드였구요!
제가 굳이 남긴 부분을 말고는 잘 하신것 같네요~

아쉬운 부분으로
다음 프로젝트때는 테스트 코드가 더 있으면 좋겠습니다. 가격계산 같은건 테스트를 잘 할 수 있고, 꼭 해야 하는 부분들이기 때문에 Price 관련 테스트가 없어서 조금 아쉬웠습니다.

수고하셨습니다!

Comment on lines +11 to +17
public static <T> ResponseBody<T> ok(T body) {
return new ResponseBody<>(body);
}

public static ResponseBody<String> notFound(String message) {
return new ResponseBody<>(message);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notFound면 status code로 구분할 수 있지 않을까요?

@@ -0,0 +1,22 @@
package com.team19.airbnb;

public class ResponseBody<T> {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

클래스가 하는 역할이 특별해 보이지 않는데 스프링에서 제공하는 ResponseEntity 를 사용하셔도 ..

Comment on lines +114 to +122
private String roomConfiguration(RoomsAndBeds roomsAndBeds) {
StringBuilder stringBuilder = new StringBuilder();
return String.valueOf(stringBuilder.append("침구 : ")
.append(roomsAndBeds.getBed())
.append(" 침실 : ")
.append(roomsAndBeds.getBedRoom())
.append(" 욕실: ")
.append(roomsAndBeds.getBathRoom()));
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 스트링으로 만들어서 주는것은 지양하면 좋습니다~
화면 요구사항이 어떻게 바뀔지 모르니 코어한 데이터로 넘겨주는 게 유연할듯하네요! (지금은 기획이 정해져있지만..)

Comment on lines +3 to +8
public class UserNotFoundException extends NotFoundException {

public UserNotFoundException() {
super("해당하는 user를 찾을 수 없습니다");
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception에 관해서는 제가 다른분들 pr 에 리뷰 남긴게 있는데 참고부탁드립니다 ㅎㅎ
#74 (comment)

Comment on lines +36 to +39
public List<Room> findPriceByAddress(Double latitude, Double longitude) {
String sql = "SELECT `price_per_day`, (6371*acos(cos(radians(?))*cos(radians(latitude))*cos(radians(longitude) -radians(?))+sin(radians(?))*sin(radians(latitude)))) AS `distance` FROM `room` HAVING `distance` <= 0.5 ORDER BY `distance`";
return jdbcTemplate.query(sql, new BeanPropertyRowMapper<>(Room.class), latitude, longitude, latitude);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 이걸 이렇게 연산하는군요.. 찾아보니 두 위경도 사이의 거리를 구하는 MySql 에서 내장 함수도 있다고 하니 적용해보시면 좋을 것 같구요~
room 이 굉장히 많고 트래픽이 몰릴때 전체를 이렇게 연산하는 작업을 어떻게 효율적으로 할지 고민도 해보세요!

  • 아왠지.. 위경도를 기반으로 저렇게 distance 공식으로 계산하지 않고, 범위를 만들어서 where절에서 가져오는건 어떨까요?
    쉽게 x,y 좌표로 보면 (1,1) 인 사람이 있으면 +1(일정둘레) 만큼 내에 있는 걸 쿼리하도록?

Comment on lines +50 to +59
if(searchRequestDTO.getCheckIn() == null && searchRequestDTO.getCheckOut() == null && searchRequestDTO.getCoordinate() == null
&& searchRequestDTO.getGuest() == null && searchRequestDTO.getMinPrice() == null && searchRequestDTO.getMaxPrice() == null) {
throw new ConditionNotFoundException();
}
if(searchRequestDTO.getCheckIn() != null && searchRequestDTO.getCheckOut() != null) {
Booking booking = searchRequestDTO.toBooking();
return roomDAO.findRoomsByCondition(searchRequestDTO).stream()
.map((room) -> new RoomDetailResponseDTO.Builder(room).totalPrice(booking.calculateTotalPrice(room.getPricePerDay())).build())
.collect(Collectors.toList());
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate 메서드 하나 빼도 되겠습니다~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants